-
Notifications
You must be signed in to change notification settings - Fork 243
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add operator hub documentation #3158
Add operator hub documentation #3158
Conversation
ping @dharmit @boczkowska |
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: The full list of commands accepted by this bot can be found here.
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
53f39f3
to
7b83f43
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Wow, that's pretty major change compared to what I had in #3029! Requested some changes.
7b83f43
to
23995f1
Compare
**What type of PR is this?** /kind documentation [skip ci] **What does does this PR do / why we need it**: Modifies the current operator hub documentation in the format as some of the other tutorial documentation **Which issue(s) this PR fixes**: N/A **How to test changes / Special notes to the reviewer**: N/A
23995f1
to
75bc1f3
Compare
Ready for another review @dharmit |
@cdrage: The following tests failed, say
Full PR test history. Your PR dashboard. Please help us cut down on flakes by linking to an open issue when you hit one in your PR. Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. I understand the commands that are listed here. |
Here we can see some placeholder data in the form of `<etcd-cluster-endpoints>` | ||
, `<aws-secret>` and `<full-s3-path>` that the user is expected to set to | ||
appropriate value for the service to start. | ||
It is important to note that `EtcdBackup` and `EtcdRestore` are **not** available for deployment as they require interactive parameters. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is not entirely correct. I mean, one can't do odo service create etcdoperator.v0.9.4 --crd EtcdBackup
or odo service create etcdoperator.v0.9.4 --crd EtcdRestore
and expect it to work OOTB. They must redirect its alm-example to a yaml, modify the yaml to contain correct values and then use the --dry-run
flag.
But, it's not worth holding this PR for that reason since --dry-run
is properly documented already.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's not entirely correct, but still a valid statement that they don't work out of the box. I don't think we should be encouraging users to have to modify a YAML, copy over metadata
annotations and then try again.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
/lgtm
Mergin' since there's one LGTM / only one needed for docs! |
What type of PR is this?
/kind documentation
[skip ci]
What does does this PR do / why we need it:
Modifies the current operator hub documentation in the format as some of
the other tutorial documentation
Which issue(s) this PR fixes:
N/A
How to test changes / Special notes to the reviewer:
N/A